Skip to content

fix(ci): ignore resolved Codex inline findings#1002

Closed
Komzpa wants to merge 1 commit into
Soju06:mainfrom
Komzpa:fix/codex-label-ignore-resolved-review-comments
Closed

fix(ci): ignore resolved Codex inline findings#1002
Komzpa wants to merge 1 commit into
Soju06:mainfrom
Komzpa:fix/codex-label-ignore-resolved-review-comments

Conversation

@Komzpa

@Komzpa Komzpa commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Ignore resolved or outdated inline Codex review findings when syncing 🤖 codex labels, so a PR can become codex: ok after the finding thread has actually been handled.

Type of change

  • fix: — bug fix (no behavior change beyond the bug)
  • feat: — new user-facing feature or capability
  • refactor: — internal refactor (no behavior change, no API change)
  • docs: — documentation only
  • chore: / ci: / build: — tooling, CI, packaging
  • test: — test-only change
  • Breaking change (also append ! after the type, e.g. feat!: or include BREAKING CHANGE: footer)

Linked issue: Related to #703

OpenSpec

  • This PR includes / updates an OpenSpec change
  • Not applicable — bug fix that matches the existing spec
  • Not applicable — docs / CI / chore only
  • This PR touches a codex-faithful path (image pipeline, request/response
    shape, SSE framing, OAuth flow) and preserves upstream-equivalent behavior

Change directory: openspec/changes/ignore-resolved-codex-inline-findings/

Changes

  • Collect unresolved, non-outdated review-thread comment URLs before merging inline review comments into the label-sync timeline.
  • Ignore current-head inline Codex finding comments when their thread has already been resolved or outdated.
  • Add regression tests proving resolved findings are ignored while unresolved findings still count.

Test plan

uv run pytest tests/unit/test_sync_codex_ok_labels.py -q
uv run ruff check .github/scripts/sync_codex_ok_labels.py tests/unit/test_sync_codex_ok_labels.py
uv run ruff format --check .github/scripts/sync_codex_ok_labels.py tests/unit/test_sync_codex_ok_labels.py
uv run openspec validate ignore-resolved-codex-inline-findings --strict
uv run openspec validate --specs

Screenshots / output (optional)

Patched dry-run against #703 before applying labels:

dry-run Soju06/codex-lb#703: head=fcf6e61cf5bd checks=success merge=CLEAN review=clean ok=False->True/add needs_work=True->False/remove legacy=- approve_runs=- trigger_codex=False reason=clean Codex review matches current head

Checklist

  • Title is in Conventional Commits format (<type>(<scope>)?: <subject>).
  • Linked the related issue / discussion above.
  • Added or updated tests covering the change.
  • Ran uv run pre-commit run local-ci --hook-stage manual --all-files or the relevant make <target> subset locally.
  • If touching specs: openspec validate --specs passes and /opsx:verify is clean.
  • CHANGELOG is not edited by hand (release-please handles it).

@Soju06

Soju06 commented Jun 14, 2026

Copy link
Copy Markdown
Owner

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12c8e080ff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@@ -0,0 +1,23 @@
## MODIFIED Requirements

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Declare the new OpenSpec requirement as added

This delta is under MODIFIED Requirements, but openspec/specs/github-automation/spec.md does not contain an existing requirement named Codex review label sync review-thread state; it currently only defines the write-token fallback and write-denial resilience requirements. OpenSpec treats MODIFIED as changes to an existing requirement, so strict validation/archive will fail for this change instead of accepting the new review-thread-state contract. Since this is a new requirement, move it under ADDED Requirements or add the matching base requirement first.

Useful? React with 👍 / 👎.

@Komzpa

Komzpa commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator Author

Folded this into #981 (d71cb6480b3249617ca4fdcbe91f4492c2123853) so the stale-thread and resolved-thread label-sync fixes can be reviewed together. Closing this PR to avoid duplicate review.

@Komzpa Komzpa closed this Jun 14, 2026
@Komzpa Komzpa deleted the fix/codex-label-ignore-resolved-review-comments branch June 15, 2026 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖 codex: needs work [@codex review] raised an issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants